Skip to content

Conversation

alkeryn
Copy link

@alkeryn alkeryn commented Jul 8, 2025

instead of hanging until the timeout and returning a PoolTimedOut error

Does your PR solve an issue?

fixes #744
this also would fix a bunch of duplicate / similar issues, didn't take the time to list them all.

Is this a breaking change?

following the last commit, no, it's a new options when creating the pool.

as mentioned by my comment in the issue :

if your db goes down you don't want users to have to wait a few seconds to get an error when they could get some 500 error immediately.

i understand that the pr may need some edits, but i wanted this issue to get some attention.

@alkeryn
Copy link
Author

alkeryn commented Jul 10, 2025

made it optional with the .return_con_refused() function that takes a bool.
this should no longer be a breaking change.

@alkeryn alkeryn force-pushed the pool_connection_refused_fix branch from 01373ef to 3e77113 Compare July 10, 2025 13:23
@alkeryn alkeryn changed the title sqlx pools immediately return ConnectionRefused errors instead of hanging and returning PoolTimedOut sqlx-core pools immediately return ConnectionRefused errors instead of hanging and returning PoolTimedOut Jul 10, 2025
@alkeryn alkeryn changed the title sqlx-core pools immediately return ConnectionRefused errors instead of hanging and returning PoolTimedOut sqlx-core pools optionally immediately return ConnectionRefused errors instead of hanging and returning PoolTimedOut Jul 10, 2025
it is now possible to set return_con_refused to true so that connection
refused errors are immediately returned instead of hanging until the
timeout and returning a PoolTimedOut error
@alkeryn alkeryn force-pushed the pool_connection_refused_fix branch from 3e77113 to 1583df2 Compare July 10, 2025 13:59
@alkeryn
Copy link
Author

alkeryn commented Aug 19, 2025

hey @abonander could this get a review ?
thank you !

@k-bx

This comment was marked as spam.

@alkeryn
Copy link
Author

alkeryn commented Oct 8, 2025

@mehcode sorry for the bump but could this get a review ?

@abonander
Copy link
Collaborator

There's kind of two orthogonal issues here:

  1. Pool::connect() et al should definitely return the connection error and not just eat it then eventually return PoolTimedOut. That's a huge UX miss on our part, I'll readily admit that. I always thought we were at least logging these errors so you could see them, but clearly we're not.
  2. You probably don't want to just immediately die on the first error, though. The Internet is messy and little hiccups happen all the time and are usually temporary. On startup, if your database server starts at the same time as your application, it's not going to be ready to accept connections right away. You don't want your deployment to be marked as failed just because of a little timing issue. And while running, if your database server does go down, you should ideally be having it run in high-availability mode with a hot standby ready to take over, in which case the next connection attempt should ideally succeed. I think most users would rather a request take a second and then succeed than immediately die with a 500.

I've been slowly working on #3582 which should bring massive improvements in this regard, including a separately settable connect_timeout. My goal is to have that make it into 0.9.0, at which point it would supercede this PR.

@alkeryn
Copy link
Author

alkeryn commented Oct 16, 2025

@abonander hey, thank you for the reply !

no, i don't necessarily want to die on the first error but i want to be able to manage it myself, there are services that if they can't connect first try, i don't want to have to wait for the whole timeout.

ie connecting to a local database when the database is down and i'm testing things, i rather have the service exit immediately than wait for no reason.

or i want to connect to another backup database if the first one failed, that kind of things.

there are also many case in which if the database actualy went offline, i want to return to the user immediately that there was an internal error and not make him wait for 30s.

i rather have the user retry than wait for nothing in many cases.

we know the connection failed, and thus i don't think that information should be hidden from the library user if he wants to be able to get it, no other databases library i know of does that.

either way, this behavior should be a choice, this is why i made it an option and i rather not have to maintain a fork just to be able to do that.

if i want to write my own retry and wait logic, or have none, i should be able to.
also if the service gets a lot of throughput, i don't think it's a good idea to keep tons of open connections when they could just return a 5xx errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PoolOptions<Postgres>::connect does not error out on connection error

3 participants